Repository navigation
Deterministic MoE Router Kernel By Removing Global Atomic Ops - #3587
Conversation
[Medium impact] The reviewed changes appear safe to merge; no blocking issue was found. SummaryAdds an optional fixed-order reduction for MoE auxiliary loss while keeping the existing atomic path as the default.
No actionable new issues were found. Tests were not run. No previous review threads were supplied, and their history could not be retrieved. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[fused_moe_aux_loss] --> B{deterministic?}
B -->|No| C[Existing atomic reduction]
B -->|Yes| D[Get workspace]
D --> E[Each block writes one partial sum]
E --> F[Fixed-order final sum]
C --> G[Loss and saved backward coefficient]
F --> G
Reviews (7) · Last reviewed commit: "Merge branch 'main' into fix/determinist..." · Reviewed by Greptile |
| * | ||
| * \return Maximum required workspace size in bytes. | ||
| */ | ||
| size_t nvte_get_moe_aux_loss_workspace_size(); |
There was a problem hiding this comment.
The way we handle the workspace in the other APIs is similar to CUB - we run the APIs with the nullptr arguments for the workspace once (which populates the other fields), then run it again with the proper workspace. Please align this API to that.
ecdbe5d to
17c35fa
Compare
|
Thanks a lot @zhongbozhu. The kernel is deterministic on one GPU. For multi-node runs, does determinism also hold with multi-node job, do we need the same placement and a fixed NCCL algorithm? Good to put it in the docs if there's anything worth noting. |
17c35fa to
07887f8
Compare
|
/te-ci pytorch |
07887f8 to
a5d4c78
Compare
|
Verified on my end:
|
|
/te-ci pytorch |
vthumbe1503
left a comment
There was a problem hiding this comment.
LGTM, with minor suggestion on workspace contract
| import transformer_engine_torch as tex | ||
| from packaging.version import Version as PkgVersion | ||
|
|
||
| import transformer_engine_torch as tex |
There was a problem hiding this comment.
This file change is not needed right?
There was a problem hiding this comment.
that seems to be linter complaint?
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com> Signed-off-by: zhongboz <zhongboz@nvidia.com>
Query workspace shape and dtype through the deterministic forward APIs, retain stream and CUDA Graph isolation, and cover metadata queries and invalid scratch buffers. Fix the stream cache-key lint warning and upstream import ordering. Validated on GB200: 201 TE tests passed (12 invalid top-k cases skipped), 6 C API tests passed, and 40 MCore checks passed with 1 expected atomic negative-control XPASS. Full PyTorch QA lint, pre-commit, and license checks passed. Signed-off-by: zhongboz <zhongboz@nvidia.com>
Signed-off-by: zhongboz <zhongboz@nvidia.com>
a5d4c78 to
23ce45f
Compare
Apply Varun Thumbe's workspace-contract changes from #1. Require the queried 256-element FP32 capacity for every input shape and update the C API workspace tests. Retain the existing MLA import grouping required by Python lint. Original author: Varun Thumbe <vthumbe@nvidia.com> Source: 9a3e764 Signed-off-by: Varun Thumbe <vthumbe@nvidia.com> Signed-off-by: zhongboz <zhongboz@nvidia.com>
|
/te-ci pytorch |
|
/te-ci L0 |
|
Relevant tests are passing. Pipeline: 77803646 |
* initial draft Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com> Signed-off-by: zhongboz <zhongboz@nvidia.com> * Align deterministic MoE aux-loss workspace queries with the common API Query workspace shape and dtype through the deterministic forward APIs, retain stream and CUDA Graph isolation, and cover metadata queries and invalid scratch buffers. Fix the stream cache-key lint warning and upstream import ordering. Validated on GB200: 201 TE tests passed (12 invalid top-k cases skipped), 6 C API tests passed, and 40 MCore checks passed with 1 expected atomic negative-control XPASS. Full PyTorch QA lint, pre-commit, and license checks passed. Signed-off-by: zhongboz <zhongboz@nvidia.com> * Test deterministic MoE aux-loss integer boundaries Signed-off-by: zhongboz <zhongboz@nvidia.com> * Simplify deterministic MoE aux-loss workspace contract Apply Varun Thumbe's workspace-contract changes from zhongbozhu#1. Require the queried 256-element FP32 capacity for every input shape and update the C API workspace tests. Retain the existing MLA import grouping required by Python lint. Original author: Varun Thumbe <vthumbe@nvidia.com> Source: zhongbozhu@9a3e764 Signed-off-by: Varun Thumbe <vthumbe@nvidia.com> Signed-off-by: zhongboz <zhongboz@nvidia.com> --------- Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com> Signed-off-by: zhongboz <zhongboz@nvidia.com> Signed-off-by: Varun Thumbe <vthumbe@nvidia.com> Co-authored-by: Varun Thumbe <vthumbe@nvidia.com> Co-authored-by: Przemyslaw Tredak <ptredak@nvidia.com> (cherry picked from commit fd2ab0c)
Description
Please include a brief summary of the changes, relevant motivation and context.
Fixes NVIDIA/Megatron-LM#7221
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: